components/groups/modals/groupDetailsModal.tsx (3)

49-49: Authors Field Display
Displaying authors in a simple paragraph works, but if multiple authors are common, consider rendering them in a list for improved readability.

---

61-61: Details Field
For very long details, consider adding truncation or a "Show More" toggle to maintain a tidy UI.

---

67-67: Proposal Forum URL
If the URL is meant to be clickable, consider wrapping it in an anchor tag to facilitate navigation.

components/groups/modals/groupInfo.tsx (2)

122-131: Mapping Authors to UI
Mapping either addresses or plain text authors is done cleanly. For improved readability, consider a dedicated function or component.

---

167-171: Centralized Metadata Parsing
Reusing JSON.parse for group metadata. Consider moving reptitive parse logic into a shared utility if it appears in multiple places.

components/groups/modals/updateGroupModal.tsx (1)

34-39: Leverage user-facing error handling for JSON parse failures
Catching JSON parse errors and logging them is helpful. However, consider providing a user-facing notification or fallback UI message (e.g., “Metadata is invalid or unavailable”) to ensure clarity for non-technical users.

components/groups/components/myGroups.tsx (3)

57-66: Keep filter logic lightweight for large group lists
While this inline JSON parsing is acceptable for small to medium datasets, you may want to parse and cache metadata once per group to avoid repeatedly parsing the same JSON string whenever the filter runs (especially for large numbers of groups).

---

82-93: Fallback name handling
Logging a warning and falling back to 'Untitled Group' is a good strategy for resilience. If the group name is crucial for user workflow, consider surfacing a more visible notification that the metadata is invalid.

---

470-476: Consistent approach to invalid metadata
Again, the fallback to 'Untitled Group' ensures consistent UX. Maintain consistency across components by employing a shared helper function to parse metadata and extract a valid name.

hooks/useQueries.ts (1)

131-131: Robust concurrency handling
Using Promise.all for policy and member results is a good approach. Ensure partial or individual promise failures don’t halt the entire fetch process if you want to retrieve data for other groups. Currently, the try/catch block handles overall failures, but a more granular error handling might be beneficial if partial data is acceptable.

Also applies to: 138-138

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/groups/forms/groups/GroupDetailsForm.tsx (1)

52-56: Consider adjusting description field validation constraints.

The current validation rules for the description field might be too restrictive:

• Minimum length of 50 characters could be excessive for simple groups
• Maximum length of 100 characters might be too limiting for detailed group descriptions

Consider relaxing these constraints, for example:

-      .min(50, 'Description must be at least 50 characters')
-      .max(100, 'Description must not exceed 100 characters')
+      .min(20, 'Description must be at least 20 characters')
+      .max(200, 'Description must not exceed 200 characters')

components/groups/modals/groupDetailsModal.tsx (2)

90-97: Simplify voting period calculation.

The current implementation involves multiple type assertions and nested property access.

Consider extracting the logic into a helper function:

const getVotingPeriodInDays = (policy: GroupPolicyInfoSDKType): string => {
  const decisionPolicy = policy?.decision_policy as ThresholdDecisionPolicySDKType;
  const seconds = decisionPolicy?.windows?.voting_period?.seconds?.toString() ?? '0';
  return Math.floor(parseInt(seconds) / 86400).toString();
};

---

165-170: Improve readability of admin status checks.

The nested ternary operators make the code harder to read and maintain.

Consider using a more readable approach:

const getAdminStatus = (address: string): React.ReactNode => {
  const isSuperAdmin = isPolicyAdmin(address) && isAdmin(address);
  const isPolicyAdminOnly = isPolicyAdmin(address) && !isAdmin(address);
  const isGroupAdminOnly = !isPolicyAdmin(address) && isAdmin(address);

  if (isSuperAdmin) return 'Super Admin';
  if (isPolicyAdminOnly) return 'Policy';
  if (isGroupAdminOnly) return 'Group';
  return <PiXCircleLight className="text-red-500 h-5 w-5" />;
};

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/groups/modals/updateGroupModal.tsx (1)

Line range hint 385-394: Fix inconsistency between input maxLength and validation

The maxLength prop on the name input (24) doesn't match the validation schema's max length (50).

  <TextInput
    label="Group Name"
    name="name"
    placeholder="Group Name"
    value={values.name}
    onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
      setName(e.target.value);
      setFieldValue('name', e.target.value);
    }}
-   maxLength={24}
+   maxLength={50}
  />

components/groups/modals/updateGroupModal.tsx (3)

34-39: Enhance error logging for metadata parsing failures

The current error handling could be more informative for debugging purposes.

  try {
    maybeMetadata = group.metadata ? JSON.parse(group.metadata) : null;
  } catch (e) {
-   console.warn('Failed to parse group metadata:', e);
+   console.warn(
+     `Failed to parse group metadata for group ${group.id}:`,
+     '\nRaw metadata:', group.metadata,
+     '\nError:', e
+   );
  }

---

Line range hint 95-157: Consider refactoring message building logic for better maintainability

The message building logic contains repeated patterns and complex conditions that could be simplified.

Consider extracting message creation into helper functions:

const createMessage = (typeUrl: string, value: Uint8Array): Any => {
  return Any.fromPartial({
    typeUrl,
    value,
  });
};

const buildMetadataMessage = (newMetadata: string) => {
  const messages: Any[] = [];
  
  // Group metadata
  const msgGroupMetadata = updateGroupMetadata({
    admin: group.admin,
    groupId: BigInt(maybeMembers?.[0]?.group_id),
    metadata: newMetadata,
  });
  messages.push(createMessage(
    cosmos.group.v1.MsgUpdateGroupMetadata.typeUrl,
    cosmos.group.v1.MsgUpdateGroupMetadata.encode(msgGroupMetadata.value).finish()
  ));

  // Policy metadata
  const msgPolicyMetadata = updateGroupPolicyMetadata({
    groupPolicyAddress: maybePolicies?.address,
    admin: group.admin,
    metadata: newMetadata,
  });
  messages.push(createMessage(
    cosmos.group.v1.MsgUpdateGroupPolicyMetadata.typeUrl,
    cosmos.group.v1.MsgUpdateGroupPolicyMetadata.encode(msgPolicyMetadata.value).finish()
  ));

  return messages;
};

---

223-292: Enhance metadata length validation

While the total metadata length validation is good, it should include all fields and provide more precise feedback.

  .test(
    'metadata-total-length',
    'Total metadata length must not exceed 255 characters',
    function (values) {
      const metadata = JSON.stringify({
        title: values.name || '',
        authors: values.authors || '',
+       summary: values.summary || '',
        details: values.description || '',
      });
-     return metadata.length <= 255;
+     const remaining = 255 - metadata.length;
+     return this.createError({
+       message: remaining < 0 
+         ? `Metadata exceeds limit by ${Math.abs(remaining)} characters`
+         : true
+     });
    }
  );

🧰 Tools

🪛 Biome (1.9.4)

[error] 242-242: Do not add then to an object.

(lint/suspicious/noThenProperty)

---

[error] 247-247: Do not add then to an object.

(lint/suspicious/noThenProperty)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

components/groups/modals/updateGroupModal.tsx (4)

34-39: Enhance error handling for metadata parsing.

While the try-catch block is good, consider adding more context to the warning message and implementing proper error recovery.

  try {
    maybeMetadata = group.metadata ? JSON.parse(group.metadata) : null;
  } catch (e) {
-   console.warn('Failed to parse group metadata:', e);
+   console.warn(`Failed to parse group metadata for group ${group.id}:`, {
+     error: e,
+     metadata: group.metadata
+   });
+   // Consider notifying the user about the parsing error
  }

---

223-268: Consider extracting validation constants and simplifying conditions.

The validation schema is complex with nested conditions. Consider extracting constants and simplifying the structure.

// Extract to constants
const MAX_NAME_LENGTH = 50;
const MAX_AUTHORS_LENGTH = 50;
const MIN_DESCRIPTION_LENGTH = 20;
const MAX_DESCRIPTION_LENGTH = 100;
const MIN_VOTING_PERIOD_MINUTES = 30;

// Simplified validation
const validationSchema = Yup.object().shape({
  name: Yup.string()
    .max(MAX_NAME_LENGTH)
    .noProfanity()
    .when(['threshold', 'votingPeriod'], {
      is: hasThresholdOrVotingPeriodChanges,
      then: schema => schema.optional(),
      otherwise: schema => schema.when(['authors', 'summary', 'description'], {
        is: hasNoMetadataChanges,
        then: schema => schema.required('At least one metadata field is required'),
        otherwise: schema => schema.optional(),
      }),
    }),
  // ... rest of the validation
});

🧰 Tools

🪛 Biome (1.9.4)

[error] 242-242: Do not add then to an object.

(lint/suspicious/noThenProperty)

---

[error] 247-247: Do not add then to an object.

(lint/suspicious/noThenProperty)

---

232-239: Extract duplicated voting period calculations.

The voting period calculations are duplicated in multiple places. Consider extracting this logic into a utility function.

// Extract to a utility function
const calculateTotalSeconds = (period: { days: number; hours: number; minutes: number; seconds: number }) => {
  return (Number(period.days) || 0) * 86400 +
         (Number(period.hours) || 0) * 3600 +
         (Number(period.minutes) || 0) * 60 +
         (Number(period.seconds) || 0);
};

Also applies to: 306-314

---

Line range hint 177-196: Enhance error handling and user feedback during form submission.

The error handling during form submission could provide better user feedback.

  } catch (error) {
    console.error('Error in handleConfirm:', error);
    setIsSigning(false);
+   // Provide user-friendly error message
+   const errorMessage = error instanceof Error ? error.message : 'An unexpected error occurred';
+   alert(errorMessage);
  }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro